Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for account-based channel bans #1546

Merged
merged 19 commits into from
Jul 24, 2024
Merged

Add support for account-based channel bans #1546

merged 19 commits into from
Jul 24, 2024

Conversation

progval
Copy link
Owner

@progval progval commented Jul 1, 2023

To keep track of network accounts using various IRCv3 specs
A future commit will add support for 'account' in
supybot.protocols.irc.banmask, but it is not supported for now,
as that config value is also used for ignore masks
And a new method .makeExtBanmask() as an alternative to .makeBanmask(),
so plugins can opt-in to extended banmasks when they support it.

'ignore' commands in Channel and anti-flood in Owner and Misc will
keep using .makeBanmask() because they use them as ignore masks in
ircdb.
Now that we can return both account extbans and regular masks,
it makes sense to ban both.

Otherwise, adding 'account' to supybot.protocols.irc.banmask
means we banned only the account instead of the hostmask,
which arguably makes the ban weaker (/NS LOGOUT to evade)
@progval progval changed the base branch from master to testing July 1, 2023 18:37
@progval progval requested a review from jlu5 July 1, 2023 18:37
src/conf.py Show resolved Hide resolved
src/conf.py Show resolved Hide resolved
plugins/Channel/test.py Outdated Show resolved Hide resolved
src/irclib.py Outdated Show resolved Hide resolved
'[email protected]')
join()
self.assertKban('kban --account foobar',
'[email protected]')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it'd be better to throw an error if only --account is given and the user's not logged in. Exact match bans are pretty trivial to evade on most networks (even accidentally if your connection times out), to the point I doubt there's much value in the bot setting them in the first place.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if it uses the default banmask instead?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it fall back to the default banmask, then to just the host (in case the default banmask is only ['account'] for some reason)

test/test_irclib.py Show resolved Hide resolved
test/test_irclib.py Show resolved Hide resolved
test/test_irclib.py Show resolved Hide resolved
test/test_irclib.py Show resolved Hide resolved
src/conf.py Outdated Show resolved Hide resolved
@progval progval changed the base branch from testing to master May 5, 2024 15:52
This only happens on the newly introduced account extban (in case the user
does not have an account, or the server does not provide accounts)
so this does not change existing behavior.

Falling back to the host instead of the exact mask makes it less easy
to evade these bans
@progval
Copy link
Owner Author

progval commented Jul 19, 2024

Should be all good now. The spec was just merged by IRCv3 (as a draft) so I'm going to merge this PR soon-ish

@progval progval merged commit 10a341c into master Jul 24, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants